Skip to content

Fix many collections implementing PartialEq#14734

Closed
tbu- wants to merge 2 commits into
rust-lang:masterfrom
tbu-:pr_eq_ne
Closed

Fix many collections implementing PartialEq#14734
tbu- wants to merge 2 commits into
rust-lang:masterfrom
tbu-:pr_eq_ne

Conversation

@tbu-

@tbu- tbu- commented Jun 7, 2014

Copy link
Copy Markdown
Contributor

They now properly check each element with the eq and ne methods in their
own eq and ne method, respectively.

@tbu-
Fix many collections implementing PartialEq
768757a 
They now properly check each element with the `eq` and `ne` methods in their
own `eq` and `ne` method, respectively.
@tbu-

tbu- commented Jun 7, 2014

Copy link
Copy Markdown
Contributor Author

See also #14733.

sfackler
sfackler reviewed Jun 8, 2014
View reviewed changes
Comment thread src/libcollections/ringbuf.rs Outdated

@sfackler sfackler Jun 8, 2014

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The && should be an ||

@heinrich5991 heinrich5991 Jun 8, 2014

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And it should be any instead of all.

@tbu-

tbu- commented Jun 8, 2014

Copy link
Copy Markdown
Contributor Author

With #14733 this will only be needed for consistency.

@alexcrichton

alexcrichton commented Jun 8, 2014

Copy link
Copy Markdown
Member

Would it be possible to add tests for this functionality? This seems very easy to get wrong during a refactoring.

@sfackler

sfackler commented Jun 8, 2014

Copy link
Copy Markdown
Member

I don't think this is necessary now that we guarantee that == and != are inverses.

@tbu-

tbu- commented Jun 8, 2014

Copy link
Copy Markdown
Contributor Author

@alexcrichton
Should I put the tests under src/tests/runpass/?

@sfackler
It's not strictly necessary – yet it would be nice to have some consistency among library defined types, e.g. always define both, or always only define eq.

@alexcrichton

alexcrichton commented Jun 8, 2014

Copy link
Copy Markdown
Member

Yes run-pass may be the best place to put them as they will likely share the same type to be checking that ne is invoked rather than eq

@alexcrichton

alexcrichton commented Jun 18, 2014

Copy link
Copy Markdown
Member

Closing due to inactivity, but feel free to reopen with a rebase!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@tbu- @alexcrichton @sfackler @heinrich5991